[V0 deprecation] Remove _VLLM_V1 suffixes from attention backend names #25489
Conversation
MatthewBonanni
requested review from
ApostaC,
DarkLight1337,
LucasWilkinson,
NickLucche,
WoosukKwon,
aarnphm,
alexm-redhat,
comaniac,
gshtras,
jikunshang,
mgoin,
njhill,
robertgshaw2-redhat,
simon-mo,
tdoublep,
tlrmchlsmth,
yewentao256 and
ywang96
as code owners
mergify
bot
added
ci/build
rocm
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively removes the _VLLM_V1 suffixes from attention backend names, which is a great cleanup now that V0 backends are deprecated. The changes are consistent across the codebase, covering test files, configuration, and backend implementations. I appreciate the addition of backward compatibility in vllm/attention/selector.py to handle the old suffixes from environment variables with a warning. I've found one minor area for improvement to simplify a redundant conditional check.
| if backend_by_env_var.endswith("_VLLM_V1"): | ||
| logger.warning( | ||
| "The suffix '_VLLM_V1' in the environment variable " | ||
| "%s is no longer necessary as V0 backends have been " | ||
| "deprecated. Please remove this suffix from your " | ||
| "environment variable setting.", STR_BACKEND_ENV_VAR) | ||
| backend_by_env_var = backend_by_env_var.removesuffix( | ||
| "_VLLM_V1") |
There was a problem hiding this comment.
@tlrmchlsmth It shouldn't be breaking; these lines should prevent issues
|
Failures are related, PTAL |
…ction logic) Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
|
The v1 tests look related too |
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Head branch was pushed to by a user without write access
|
@mgoin Thanks - I removed that test case too (there are no longer any V0 backends to fall back to). Eventually we'll have to remove |
|
Created #25673 to remove the oracle |
|
@mgoin V1 Test e2e + engine is failing for backend |
|
@MatthewBonanni is it possible to fix the test? Otherwise you can make an issue and disable it |
|
@ProExpertProg It looks to me like a correctness issue with the FI backend outside the scope of this PR, so I'll disable it and make an issue. Disabling it shouldn't do too much harm because it was effectively disabled before. EDIT: The issue is #25679 |
vllm-project/vllm#25489 Signed-off-by: Chendi Xue <Chendi.Xue@intel.com>
vllm-project/vllm#25489 Signed-off-by: Chendi Xue <Chendi.Xue@intel.com> Signed-off-by: Iryna Boiko <iboiko@habana.ai>
#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com> Signed-off-by: yewentao256 <zhyanwentao@126.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com>
vllm-project#25489) Signed-off-by: Matthew Bonanni <mbonanni@redhat.com> Signed-off-by: Matthew Bonanni <mbonanni001@gmail.com> Signed-off-by: xuebwang-amd <xuebwang@amd.com>
4 participants
Purpose
Remove
_VLLM_V1suffixes as these are no longer needed. If a user includes that suffix in theirVLLM_ATTENTION_BACKENDenvironment variable, it is stripped and a warning is printed.Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.